Skip to content

fix(browser): export saved files with write output - #419

Open
gwelinder wants to merge 2 commits into
steipete:mainfrom
gwelinder:fix/frog-sandbox-artifact-write-output-20260826
Open

fix(browser): export saved files with write output#419
gwelinder wants to merge 2 commits into
steipete:mainfrom
gwelinder:fix/frog-sandbox-artifact-write-output-20260826

Conversation

@gwelinder

@gwelinder gwelinder commented Aug 26, 2026

Copy link
Copy Markdown

Summary

  • carry browser savedFiles through the session-runner boundary
  • copy captured sandbox/download artifacts beside the requested --write-output file
  • preserve canonical session artifacts and binary bytes
  • validate adjacent copies by size and SHA-256
  • avoid overwrites with deterministic exclusive collision naming
  • persist explicit warnings when an adjacent copy fails

Validation

  • focused browser/CLI/remote tests: 5 files, 159 tests passed
  • pnpm run check: formatting, TypeScript, and oxlint passed
  • mocked regression covers a response containing only sandbox:/mnt/data/report.md, exact bytes/hash, collision behavior, and copy failure
  • independent review passed

No Chrome or live browser was launched. The branch includes a normal merge of current upstream main; no rebase or force-push was used.

@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 26, 2026
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 25, 2026, 8:18 PM ET / August 26, 2026, 00:18 UTC.

ClawSweeper review

What this changes

The PR forwards browser-captured downloadable files to the CLI and copies verified files beside a requested --write-output answer using collision-safe names.

Merge readiness

⛔ Blocked until real behavior proof is added - 3 items remain

Keep open pending required real-behavior proof. The source change is focused and appears to repair the dropped saved-file boundary, but the PR explicitly reports that no Chrome or live browser was run.

Priority: P2
Reviewed head: 6d21d55a00c796821842a4a594cf2d0b0e296a69

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation and focused coverage look solid, but the required real browser behavior evidence is absent.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: No after-fix real behavior proof is present: the PR explicitly says no Chrome or live browser ran. Please attach a redacted terminal transcript or recording showing a browser-generated file copied beside --write-output, including the recorded hash; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: No after-fix real behavior proof is present: the PR explicitly says no Chrome or live browser ran. Please attach a redacted terminal transcript or recording showing a browser-generated file copied beside --write-output, including the recorded hash; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Saved-file boundary repair: The browser execution result now preserves savedFiles, allowing the CLI to receive files that current main otherwise retains only in the browser result.
Copy safeguards: Adjacent copies use exclusive creation, then validate byte size and SHA-256; failures remove the incomplete copy and persist a browser warning.
Focused regression coverage: The CLI test covers exact bytes and hash, collision naming, preservation of the canonical artifact, and non-fatal copy failure.
Findings None None.
Security None None.

Live Verification

Command: pnpm run oracle -- --help

Result: PASS (completed)

$ pnpm start -- --help
$ pnpm run build && node ./dist/scripts/run-cli.js -- --help
$ tsc -p tsconfig.build.json && pnpm run build:vendor
$ node -e "const fs=require('fs'); const path=require('path'); const vendorRoot=path.join('dist','vendor'); fs.rmSync(vendorRoot,{recursive:true,force:true}); c
onst vendors=[['oracle-notifier']]; vendors.forEach(([name])=›{const src=path.join('vendor',name); const dest=path.join(vendorRoot,name); fs.mkdirSync(dest,{rec
ursive:true}); if(fs.existsSync(src)){fs.cpSync(src,dest,{recursive:true,force:true});}});"
Oracle CLI v0.18.0 — Prompt + files required — GPT-5.5 Pro/GPT-5.5 for tough questions with code/file context.

Usage: oracle [options] [command] [prompt]

One-shot GPT-5.5 Pro / GPT-5.5 / GPT-5.1 Codex tool for hard questions that benefit from large file context and server-side search.

Arguments:
  prompt                             Prompt text (shorthand for --prompt).

Options:
  -V, --version                      output the version number
  -p, --prompt ‹text›                User prompt to send to the model.
  --followup ‹sessionId|responseId›  Continue a stored ChatGPT browser conversation or an OpenAI/Azure Responses API run.
  --followup-model ‹model›           For multi-model API sessions, choose which model response to continue from.
  -f, --file ‹paths...›              Files/directories or glob patterns to attach (prefix with !pattern to exclude). Oversized files are rejected automatically
                                     (default cap: 1 MB; configurable via ORACLE_MAX_FILE_SIZE_BYTES or config.maxFileSizeBytes). (default: [])
  --max-file-size-bytes ‹bytes›      Reject files larger than this many bytes.
  --copy-markdown                    Copy the assembled markdown bundle to the clipboard; pair with --render to print it too. (default: false)
  -s, --slug ‹words›                 Custom session slug (3-5 words).
  -m, --model ‹model›                Model to target (gpt-5.5-pro default). GPT-5.6 aliases gpt-5.6 and gpt-5.6-sol work with the OpenAI API or ChatGPT browser.
                                     In browser mode, generic Pro aliases follow the current GPT-5.6 Sol target; use explicit gpt-5.5-pro to pin GPT-5.5.
                                     Retired GPT-5.2 base/Instant/Thinking aliases are API-only. Other API targets include gpt-5.1-codex, gpt-5.2,
                                     gpt-5.2-instant, Gemini, Claude, and custom model IDs.
  --models ‹models›                  Comma-separated API model list to query in parallel (e.g., "gpt-5.5-pro,gemini-3-pro"). (default: [])
  --reasoning-effort ‹effort›        Reasoning effort for GPT-5.6 API models. (choices: "none", "low", "medium", "high", "xhigh", "max")
  --reasoning-mode ‹mode›            Responses API reasoning execution mode for GPT-5.6 models ("standard" or "pro"). (choices: "standard", "pro")
  -e, --engine ‹mode›                Execution engine (api | browser). Browser engine: GPT models automate ChatGPT; Gemini models use a cookie-based client for
                                     gemini.google.com. If omitted, oracle picks api when OPENAI_API_KEY is set, otherwise browser. (choices: "api", "browser")
  --files-report                     Show token usage per attached file (also prints automatically when files exceed the token budget). (default: false)
  -v, --verbose                      Enable verbose logging for all operations. (default: false)
  --notify                           Desktop notification when a session finishes (default on unless CI/SSH).
  --no-notify                        Disable desktop notifications.
  --notify-sound                     Play a notification sound on completion (default off).
  --no-notify-sound                  Disable notification sounds.
  --timeout ‹seconds|duration|auto›  Overall timeout before aborting the API call (auto = 60m for Pro models, 120s otherwise). (default: "auto")
  --background                       Use Respo
… output truncated …

Assertions:

  • PASS expect_output: Usage: oracle

How this fits together

Oracle’s browser runner captures ChatGPT response artifacts, then returns them through the session runner to the CLI. The CLI writes the answer, records session metadata, and now also places verified file artifacts beside that answer.

flowchart LR
  A[ChatGPT browser response] --> B[Artifact capture]
  B --> C[Browser session runner]
  C --> D[CLI session runner]
  D --> E[Write answer output]
  D --> F[Copy verified saved files]
  F --> G[Session artifacts and warnings]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: No after-fix real behavior proof is present: the PR explicitly says no Chrome or live browser ran. Please attach a redacted terminal transcript or recording showing a browser-generated file copied beside --write-output, including the recorded hash; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - A mocked regression does not show that an authenticated browser download reaches the CLI and appears beside --write-output in a real run.
  • Complete next step (P2) - The remaining merge blocker is contributor-supplied real behavior proof, not a mechanical repair for the automation lane.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface production +119, tests +140, changelog +1 Most of the patch is focused regression coverage for a small browser-to-CLI artifact handoff.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Retain the typed handoff and verified collision-safe copies, then demonstrate a real browser-generated file copied beside the requested output without exposing account data or private paths.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the typed handoff and verified collision-safe copies, then demonstrate a real browser-generated file copied beside the requested output without exposing account data or private paths.

Do we have a high-confidence way to reproduce the issue?

Yes—source-reproducible: current main captures savedFiles but does not return them through the browser session result, so the CLI cannot export them beside --write-output. A live browser run has not been supplied.

Is this the best way to solve the issue?

Yes—the typed handoff plus exclusive copy, byte/hash verification, and warning-on-failure is a narrow maintainable repair; it still needs after-fix behavior proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 79e483bd9dc8.

Labels

Label changes:

  • add P2: This is a bounded artifact-export reliability fix that needs normal merge review and real behavior evidence.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present: the PR explicitly says no Chrome or live browser ran. Please attach a redacted terminal transcript or recording showing a browser-generated file copied beside --write-output, including the recorded hash; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded artifact-export reliability fix that needs normal merge review and real behavior evidence.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present: the PR explicitly says no Chrome or live browser ran. Please attach a redacted terminal transcript or recording showing a browser-generated file copied beside --write-output, including the recorded hash; updating the PR body should trigger re-review, or ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Saved-file boundary repair: The browser execution result now preserves savedFiles, allowing the CLI to receive files that current main otherwise retains only in the browser result. (src/browser/sessionRunner.ts:43, 6d21d55a00c7)
  • Copy safeguards: Adjacent copies use exclusive creation, then validate byte size and SHA-256; failures remove the incomplete copy and persist a browser warning. (src/cli/sessionRunner.ts:1105, 6d21d55a00c7)
  • Focused regression coverage: The CLI test covers exact bytes and hash, collision naming, preservation of the canonical artifact, and non-fatal copy failure. (tests/cli/sessionRunner.test.ts:1126, 6d21d55a00c7)
  • Current artifact provenance: Current-main blame attributes the SavedBrowserFile contract to the v0.17.3 release commit, and history shows Peter Steinberger as the dominant contributor across the affected artifact and session paths. (src/browser/types.ts:56, 6b17e6db0cae)
  • Proof gap: The PR body says that no Chrome or live browser was launched; its reported tests are mocked and supplemental rather than after-fix behavior proof. (6d21d55a00c7)

Likely related people:

  • Peter Steinberger: Current-main blame ties the saved-file contract to the v0.17.3 release commit, and the feature-history shortlog shows extensive work across the affected browser artifact and CLI session files. (role: recent artifact and session-path contributor; confidence: high; commits: 6b17e6db0cae; files: src/browser/types.ts, src/browser/chatgptFiles.ts, src/cli/sessionRunner.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Post redacted after-fix output from a browser run that generates a downloadable file and show the adjacent copied file and SHA-256.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gwelinder